-
Notifications
You must be signed in to change notification settings - Fork 8.1k
drivers: i2s: stm32 sai add support for stm32f4xx series #97829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
aa80eee
to
5ba3553
Compare
5ba3553
to
900fbe6
Compare
These changes enable SAI1 A & B nodes in STM32F4xx series. Signed-off-by: Mario Paja <[email protected]>
STM32F4xx series shares several DMA configurations with the other platforms. These changes aim to enable platform specific DMA configuration and align them to other platforms. Signed-off-by: Mario Paja <[email protected]>
Add nucleo_f429zi board in samples/drivers/i2s/output Signed-off-by: Mario Paja <[email protected]>
900fbe6
to
cc72746
Compare
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have several suggestions to replace the CONFIG_SOC_SERIES_XXX
macros. I think it would ease the porting of other series featuring the SAI, and would also lighten the code a bit.
#if !defined(CONFIG_SOC_SERIES_STM32H7X) && !defined(CONFIG_SOC_SERIES_STM32L4X) && \ | ||
!defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) | ||
!defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) && \ | ||
!defined(CONFIG_SOC_SERIES_STM32F4X) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we replace all these defined with a single #if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_dma)
?
That would probably make adding other series easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will do it on a separate PR after F7 is merged (and then I am node with almost all series), It is already in todo :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very well. Approved!
#if !defined(CONFIG_SOC_SERIES_STM32H7X) && !defined(CONFIG_SOC_SERIES_STM32L4X) && \ | ||
!defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) | ||
!defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) && \ | ||
!defined(CONFIG_SOC_SERIES_STM32F4X) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
#elif !defined(CONFIG_SOC_SERIES_STM32H7X) && !defined(CONFIG_SOC_SERIES_STM32L4X) && \ | ||
!defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) | ||
!defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) && \ | ||
!defined(CONFIG_SOC_SERIES_STM32F4X) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
#if defined(CONFIG_SOC_SERIES_STM32H7X) || defined(CONFIG_SOC_SERIES_STM32L4X) || \ | ||
defined(CONFIG_SOC_SERIES_STM32G4X) || defined(CONFIG_SOC_SERIES_STM32L5X) | ||
defined(CONFIG_SOC_SERIES_STM32G4X) || defined(CONFIG_SOC_SERIES_STM32L5X) || \ | ||
defined(CONFIG_SOC_SERIES_STM32F4X) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here we could maybe use #if DT_HAS_COMPAT_STATUS_OKAY(st_stm32_dma_v1) || DT_HAS_COMPAT_STATUS_OKAY(st_stm32_dma_v2)
?
/* Control of MCLK output from SAI configuration is not possible on STM32L4xx MCUs */ | ||
#if !defined(CONFIG_SOC_SERIES_STM32L4X) | ||
#if !defined(CONFIG_SOC_SERIES_STM32L4X) && !defined(CONFIG_SOC_SERIES_STM32F4X) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment is outdated (missing F4). This could be replaced with #ifdef SAI_MCK_OUTPUT_ENABLE
to avoid having to add other series in future.
} | ||
|
||
hdma->Instance = STM32_DMA_GET_INSTANCE(stream->reg, stream->dma_channel); | ||
#if defined(CONFIG_SOC_SERIES_STM32F4X) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it could probably be #ifdef DMA_CHANNEL_1
.
#endif | ||
|
||
#if defined(CONFIG_SOC_SERIES_STM32H7X) | ||
#if defined(CONFIG_SOC_SERIES_STM32H7X) || defined(CONFIG_SOC_SERIES_STM32F4X) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here #ifdef DMA_FIFOMODE_DISABLE
/* MckOverSampling is not supported by all STM32L4xx MCUs */ | ||
#if !defined(CONFIG_SOC_SERIES_STM32L4X) | ||
#if !defined(CONFIG_SOC_SERIES_STM32L4X) && !defined(CONFIG_SOC_SERIES_STM32F4X) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. Could be replaced with #ifdef SAI_MCK_OVERSAMPLING_DISABLE
This PR enables SAI on STM32F4xx series by:
Audio Test:
2CH, 16bit, 44.1KHz, PCM5102a
stm32f4xx_sai.mp4